Track scroll event handlers in nested documents

Source/core/page/EventHandlerRegistry.cpp

Index: Source/core/page/EventHandlerRegistry.cpp
diff --git a/Source/core/dom/EventHandlerRegistry.cpp b/Source/core/page/EventHandlerRegistry.cpp
similarity index 57%
rename from Source/core/dom/EventHandlerRegistry.cpp
rename to Source/core/page/EventHandlerRegistry.cpp
index b72b465ffaadc0a31187c14559227570e38b81e7..b43ec9cbdf5c2454370dd3ecac0512f9fb40d11d 100644
--- a/Source/core/dom/EventHandlerRegistry.cpp
+++ b/Source/core/page/EventHandlerRegistry.cpp
@@ -3,30 +3,15 @@
 // found in the LICENSE file.
 
 #include "config.h"
-#include "core/dom/EventHandlerRegistry.h"
+#include "core/page/EventHandlerRegistry.h"
 
-#include "core/dom/Document.h"
 #include "core/events/ThreadLocalEventNames.h"
-#include "core/events/WheelEvent.h"
-#include "core/frame/FrameHost.h"
-#include "core/frame/LocalFrame.h"
-#include "core/page/Chrome.h"
-#include "core/page/ChromeClient.h"
-#include "core/page/Page.h"
 #include "core/page/scrolling/ScrollingCoordinator.h"
 
 namespace WebCore {
 
-EventHandlerRegistry::HandlerState::HandlerState()
-{
-}
-
-EventHandlerRegistry::HandlerState::~HandlerState()
-{
-}
-
-EventHandlerRegistry::EventHandlerRegistry(Document& document)
-    : m_document(document)
+EventHandlerRegistry::EventHandlerRegistry(Page& page)
+    : m_page(page)
 {
 }
 
@@ -39,12 +24,12 @@ const char* EventHandlerRegistry::supplementName()
     return "EventHandlerRegistry";
 }
 
-EventHandlerRegistry* EventHandlerRegistry::from(Document& document)
+EventHandlerRegistry* EventHandlerRegistry::from(Page& page)
 {
-    EventHandlerRegistry* registry = static_cast<EventHandlerRegistry*>(DocumentSupplement::from(document, supplementName()));
+    EventHandlerRegistry* registry = static_cast<EventHandlerRegistry*>(Supplement<Page>::from(page, supplementName()));
     if (!registry) {
-        registry = new EventHandlerRegistry(document);
-        DocumentSupplement::provideTo(document, supplementName(), adoptPtr(registry));
+        registry = new EventHandlerRegistry(page);
+        Supplement<Page>::provideTo(page, supplementName(), adoptPtr(registry));
     }
     return registry;
 }
@@ -61,40 +46,25 @@ bool EventHandlerRegistry::eventTypeToClass(const AtomicString& eventType, Event
 
 const EventTargetSet* EventHandlerRegistry::eventHandlerTargets(EventHandlerClass handlerClass) const
 {
-    return m_eventHandlers[handlerClass].targets.get();
+    return &m_targets[handlerClass];
 }
 
 bool EventHandlerRegistry::hasEventHandlers(EventHandlerClass handlerClass) const
 {
-    EventTargetSet* targets = m_eventHandlers[handlerClass].targets.get();
-    return targets && targets->size();
+    return m_targets[handlerClass].size();
 }
 
 bool EventHandlerRegistry::updateEventHandlerTargets(ChangeOperation op, EventHandlerClass handlerClass, EventTarget* target)
 {
-    EventTargetSet* targets = m_eventHandlers[handlerClass].targets.get();
+    EventTargetSet* targets = &m_targets[handlerClass];
     if (op == Add) {
-#if ASSERT_ENABLED
-        if (Node* node = target->toNode())
-            ASSERT(&node->document() == &m_document);
-#endif // ASSERT_ENABLED
-
-        if (!targets) {
-            m_eventHandlers[handlerClass].targets = adoptPtr(new EventTargetSet);
-            targets = m_eventHandlers[handlerClass].targets.get();
-        }
-
         if (!targets->add(target).isNewEntry) {
             // Just incremented refcount, no real change.
             return false;
         }
     } else {
-        // Note that we can't assert that |target| is in this document because
-        // it might be in the process of moving out of it.
         ASSERT(op == Remove || op == RemoveAll);
         ASSERT(op == RemoveAll || targets->contains(target));
-        if (!targets)
-            return false;
 
         if (op == RemoveAll) {
             if (!targets->contains(target))
@@ -112,19 +82,11 @@ bool EventHandlerRegistry::updateEventHandlerTargets(ChangeOperation op, EventHa
 
 void EventHandlerRegistry::updateEventHandlerInternal(ChangeOperation op, EventHandlerClass handlerClass, EventTarget* target)
 {
-    // After the document has stopped, all updates become no-ops.
-    if (!m_document.isActive()) {
-        return;
-    }
-
     bool hadHandlers = hasEventHandlers(handlerClass);
     updateEventHandlerTargets(op, handlerClass, target);
     bool hasHandlers = hasEventHandlers(handlerClass);
 
-    // Notify the parent document's registry if we added the first or removed
-    // the last handler.
-    if (hadHandlers != hasHandlers && !m_document.parentDocument()) {
-        // This is the root registry; notify clients accordingly.
+    if (hadHandlers != hasHandlers) {
         notifyHasHandlersChanged(handlerClass, hasHandlers);
     }
 }
@@ -157,36 +119,45 @@ void EventHandlerRegistry::didRemoveEventHandler(EventTarget& target, EventHandl
     updateEventHandlerInternal(Remove, handlerClass, &target);
 }
 
-void EventHandlerRegistry::didMoveFromOtherDocument(EventTarget& target, Document& oldDocument)
+void EventHandlerRegistry::didMoveIntoPage(EventTarget& target)
 {
-    EventHandlerRegistry* oldRegistry = EventHandlerRegistry::from(oldDocument);
-    for (size_t i = 0; i < EventHandlerClassCount; ++i) {
-        EventHandlerClass handlerClass = static_cast<EventHandlerClass>(i);
-        const EventTargetSet* targets = oldRegistry->eventHandlerTargets(handlerClass);
-        if (!targets)
-            continue;
-        for (unsigned count = targets->count(&target); count > 0; --count) {
-            oldRegistry->updateEventHandlerInternal(Remove, handlerClass, &target);
-            updateEventHandlerInternal(Add, handlerClass, &target);
-        }
-    }
+    if (Node* node = target.toNode())
+        updateEventHandlersRecursively(Add, node);
+}
+
+void EventHandlerRegistry::didMoveOutOfPage(EventTarget& target)
+{
+    if (Node* node = target.toNode())
+        updateEventHandlersRecursively(RemoveAll, node);
 }
 
 void EventHandlerRegistry::didRemoveAllEventHandlers(EventTarget& target)
 {
     for (size_t i = 0; i < EventHandlerClassCount; ++i) {
         EventHandlerClass handlerClass = static_cast<EventHandlerClass>(i);
-        const EventTargetSet* targets = eventHandlerTargets(handlerClass);
-        if (!targets)
-            continue;
         updateEventHandlerInternal(RemoveAll, handlerClass, &target);
     }
 }
 
+void EventHandlerRegistry::updateEventHandlersRecursively(ChangeOperation op, Node* node)
+{
+    if (node->hasEventListeners()) {
+        Vector<AtomicString> eventTypes = node->eventTypes();
+        for (size_t i = 0; i < eventTypes.size(); ++i) {
+            EventHandlerClass handlerClass;
+            if (!eventTypeToClass(eventTypes[i], &handlerClass))
+                continue;
+            for (unsigned count = node->getEventListeners(eventTypes[i]).size(); count > 0; --count)

[Rick Byers, 2014/04/28 21:18:07]

Why call this in a loop for RemoveAll?  Ideally we'd be able to set the ref
count directly rather than looping for the Add case too, but that might not be
worth complexity.

[Sami, 2014/04/29 14:21:28]

Right, I wasn't sure if a fast path RemoveAll was worth the hassle, but I've now
added it and it isn't too bad.

Looks like HashCountedSet doesn't support bulk additions, so doing the same for
additions would just be moving the loop further down. I'm inclined to keep the
loop here since this is the only case we need it for. 

+                updateEventHandlerInternal(op, handlerClass, node);
+        }
+    }
+    for (Node* child = node->firstChild(); child; child = child->nextSibling())

[Rick Byers, 2014/04/28 21:18:07]

I don't think you want to do this recursively.  It looks to me like
Node::didMoveToNewDocument is called for each node that moved (see iteration in
TreeScopeAdopter::moveTreeToNewScope) so this will count each node other than
the root multiple times and be very expensive.  Ideally we'd have a test that
verifies this.

[Sami, 2014/04/29 14:21:28]

Oh, thanks for pointing this out! Definitely don't want to be doing recursive
recursion :P I checked that the recursion was needed while writing the patch,
but looks like I got it somehow wrong. I've now fixed this and added a test.

+        updateEventHandlersRecursively(op, child);
+}
+
 void EventHandlerRegistry::notifyHasHandlersChanged(EventHandlerClass handlerClass, bool hasActiveHandlers)
 {
-    Page* page = m_document.page();
-    ScrollingCoordinator* scrollingCoordinator = page ? page->scrollingCoordinator() : 0;
+    ScrollingCoordinator* scrollingCoordinator = m_page.scrollingCoordinator();
 
     switch (handlerClass) {
     case ScrollEvent: