Change EventTarget callback APIs for add/RemoveEventListenerInternal.

third_party/WebKit/Source/core/events/EventListenerMap.cpp

Index: third_party/WebKit/Source/core/events/EventListenerMap.cpp
diff --git a/third_party/WebKit/Source/core/events/EventListenerMap.cpp b/third_party/WebKit/Source/core/events/EventListenerMap.cpp
index 5480595cc48759e793ecae240f1ace6488af2302..1eaa05f752fc9218e93e46ca909bcfc6ed576929 100644
--- a/third_party/WebKit/Source/core/events/EventListenerMap.cpp
+++ b/third_party/WebKit/Source/core/events/EventListenerMap.cpp
@@ -80,7 +80,7 @@ bool EventListenerMap::containsCapturing(const AtomicString& eventType) const
     for (const auto& entry : m_entries) {
         if (entry.first == eventType) {
             for (const auto& eventListener: *entry.second) {
-                if (eventListener.useCapture)
+                if (eventListener.capture())
                     return true;
             }
         }
@@ -106,47 +106,52 @@ Vector<AtomicString> EventListenerMap::eventTypes() const
     return types;
 }
 
-static bool addListenerToVector(EventListenerVector* vector, EventListener* listener, const EventListenerOptions& options)
+static bool addListenerToVector(EventListenerVector* vector, EventListener* listener, const EventListenerOptions& options, RegisteredEventListener* registeredListener)
 {
-    RegisteredEventListener registeredListener(listener, options);
+    *registeredListener = RegisteredEventListener(listener, options);
 
-    if (vector->find(registeredListener) != kNotFound)
+    if (vector->find(*registeredListener) != kNotFound)
         return false; // Duplicate listener.
 
-    vector->append(registeredListener);
+    vector->append(*registeredListener);
     return true;
 }
 
-bool EventListenerMap::add(const AtomicString& eventType, EventListener* listener, const EventListenerOptions& options)
+bool EventListenerMap::add(const AtomicString& eventType, EventListener* listener, const EventListenerOptions& options, RegisteredEventListener* registeredListener)
 {
     assertNoActiveIterators();
 
     for (const auto& entry : m_entries) {
         if (entry.first == eventType)
-            return addListenerToVector(entry.second.get(), listener, options);
+            return addListenerToVector(entry.second.get(), listener, options, registeredListener);
     }
 
     m_entries.append(std::make_pair(eventType, new EventListenerVector));
-    return addListenerToVector(m_entries.last().second.get(), listener, options);
+    return addListenerToVector(m_entries.last().second.get(), listener, options, registeredListener);
 }
 
-static bool removeListenerFromVector(EventListenerVector* listenerVector, EventListener* listener, const EventListenerOptions& options, size_t& indexOfRemovedListener)
+static bool removeListenerFromVector(EventListenerVector* listenerVector, const EventListener* listener, const EventListenerOptions& options, size_t& indexOfRemovedListener, RegisteredEventListener* registeredListener)

[bokan, 2016/05/03 15:52:46]

Pass indexOfRemovedListener by pointer since it's an out param
(https://google.github.io/styleguide/cppguide.html#Reference_Arguments)

[dtapuska, 2016/05/03 20:53:04]

Done.

 {
-    RegisteredEventListener registeredListener(listener, options);
-    indexOfRemovedListener = listenerVector->find(registeredListener);
-    if (indexOfRemovedListener == kNotFound)
-        return false;
-    listenerVector->remove(indexOfRemovedListener);
-    return true;
+    indexOfRemovedListener = 0;
+    for (const auto& eventListener : *listenerVector) {

[bokan, 2016/05/03 15:52:46]

Why did you have to transform vector->find into a hand rolled loop?

[dtapuska, 2016/05/03 16:23:24]

This was done to correct the const behaviour. EventListener* listener can be
const EventListener* on the remove except for it's usage here if it was to
create a RegisteredEventListener object.

[bokan, 2016/05/03 17:42:02]

Thanks, makes sense. IMO, this would be clearer with std::find_if but you can
judge (e.g.
https://code.google.com/p/chromium/codesearch/#chromium/src/third_party/WebKi...)

[dtapuska, 2016/05/03 20:53:04]

Done.

+        if (eventListener.matches(listener, options)) {
+            *registeredListener = eventListener;
+            listenerVector->remove(indexOfRemovedListener);
+            return true;
+        }
+        ++indexOfRemovedListener;
+    }
+    indexOfRemovedListener = kNotFound;
+    return false;
 }
 
-bool EventListenerMap::remove(const AtomicString& eventType, EventListener* listener, const EventListenerOptions& options, size_t& indexOfRemovedListener)
+bool EventListenerMap::remove(const AtomicString& eventType, const EventListener* listener, const EventListenerOptions& options, size_t& indexOfRemovedListener, RegisteredEventListener* registeredListener)
 {
     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, options, indexOfRemovedListener);
+            bool wasRemoved = removeListenerFromVector(m_entries[i].second.get(), listener, options, indexOfRemovedListener, registeredListener);
             if (m_entries[i].second->isEmpty())
                 m_entries.remove(i);
             return wasRemoved;
@@ -170,12 +175,12 @@ EventListenerVector* EventListenerMap::find(const AtomicString& eventType)
 
 static void copyListenersNotCreatedFromMarkupToTarget(const AtomicString& eventType, EventListenerVector* listenerVector, EventTarget* target)
 {
-    for (const auto& eventListener : *listenerVector) {
+    for (auto& eventListener : *listenerVector) {
         // Event listeners created from markup have already been transfered to the shadow tree during cloning.
-        if (eventListener.listener->wasCreatedFromMarkup())
+        if (eventListener.listener()->wasCreatedFromMarkup())
             continue;
         EventListenerOptions options = eventListener.options();
-        target->addEventListener(eventType, eventListener.listener, options);
+        target->addEventListener(eventType, eventListener.listener(), options);
     }
 }
 
@@ -231,7 +236,7 @@ EventListener* EventListenerIterator::nextListener()
     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;
     }