Track scroll event handlers in nested documents

Source/core/page/EventHandlerRegistry.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // 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::EventHandlerRegistry(Page& page)
+    : m_page(page)
 {
 }
 
-EventHandlerRegistry::HandlerState::~HandlerState()
-{
-}
-
-EventHandlerRegistry::EventHandlerRegistry(Document& document)
-    : m_document(document)
-{
-}
-
 EventHandlerRegistry::~EventHandlerRegistry()
 {
 }
 
 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;
 }
 
 bool EventHandlerRegistry::eventTypeToClass(const AtomicString& eventType, EventHandlerClass* result)
 {
     if (eventType == EventTypeNames::scroll) {
         *result = ScrollEvent;
     } else {
         return false;
     }
     return true;
 }
 
 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))
                 return false;
             targets->removeAll(target);
         } else {
             if (!targets->remove(target)) {
                 // Just decremented refcount, no real update.
                 return false;
             }
         }
     }
     return true;
 }
 
 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);
     }
 }
 
 void EventHandlerRegistry::updateEventHandlerOfType(ChangeOperation op, const AtomicString& eventType, EventTarget* target)
 {
     EventHandlerClass handlerClass;
     if (!eventTypeToClass(eventType, &handlerClass))
         return;
     updateEventHandlerInternal(op, handlerClass, target);
@@ skipping 12 matching lines @@
 void EventHandlerRegistry::didAddEventHandler(EventTarget& target, EventHandlerClass handlerClass)
 {
     updateEventHandlerInternal(Add, handlerClass, &target);
 }
 
 void EventHandlerRegistry::didRemoveEventHandler(EventTarget& target, EventHandlerClass handlerClass)
 {
     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:
         if (scrollingCoordinator)
             scrollingCoordinator->updateHaveScrollEventHandlers();
         break;
     default:
         ASSERT_NOT_REACHED();
         break;
     }
 }
 
 } // namespace WebCore