Add EventHandlerRegistry

Source/core/dom/EventHandlerRegistry.h

+// 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.
+#ifndef EventHandlerRegistry_h
+#define EventHandlerRegistry_h
+
+#include "core/dom/DocumentLifecycleObserver.h"
+#include "core/dom/DocumentSupplementable.h"
+#include "core/events/Event.h"
+#include "core/frame/DOMWindowLifecycleObserver.h"
+#include "wtf/HashCountedSet.h"
+
+namespace WebCore {
+
+typedef HashCountedSet<EventTarget*> EventTargetSet;
+
+// Registry for keeping track of event handlers. Handlers can either be
+// associated with an EventTarget or be "external" handlers which live outside
+// the DOM (e.g., WebViewImpl).
+class EventHandlerRegistry FINAL : public DocumentSupplement {
+public:
+    virtual ~EventHandlerRegistry();
+
+    // Supported event handler classes. Note that each one may correspond to
+    // multiple event types.
+    enum EventHandlerClass {
+        WheelEvent,
+        ScrollEvent,
+        TouchEvent,
+        EventHandlerClassCount, // Must be the last entry.
+    };
+
+    static const char* supplementName();
+    static EventHandlerRegistry* from(Document&);
+
+    // Returns true if the host Document or any child documents have any
+    // registered event handlers of the class.
+    bool hasEventHandlers(EventHandlerClass) const;
+
+    // Returns a set of EventTargets which have registered handlers of the
+    // given class. Only contains targets directly in this document; all
+    // handlers in a child Document are collapsed to a single respective
+    // Document instance in the set.
+    const EventTargetSet* eventHandlerTargets(EventHandlerClass) const;
+
+    // Returns the number of external event handlers for a given class.
+    // External handlers are not included in |eventHandlerTargets|.
+    unsigned externalEventHandlerCount(EventHandlerClass) const;
+
+    // Registration and management of event handlers attached to EventTargets.
+    void didAddEventHandler(EventTarget&, const AtomicString& eventType);
+    void didAddEventHandler(EventTarget&, EventHandlerClass);
+    void didRemoveEventHandler(EventTarget&, const AtomicString& eventType);
+    void didRemoveEventHandler(EventTarget&, EventHandlerClass);
+    void didMoveToNewDocument(EventTarget&, Document& oldDocument);

[Rick Byers, 2014/04/03 15:33:19]

Nit: A call like "doc1.didMoveToNewDocument(t, doc2)" reads to me like t moved
from doc1 to doc2 (not from doc2 to doc1).  Perhaps rename it to
'didMoveFromOtherDocument'?
  
Alternately you could change the implementation to match the name (looks like it
should be similar either way).

Or I guess if there's really no strong association with one document over the
other, then it could be a static method like didMoveToNewDocument(target,
oldDoc, newDoc)

[Sami, 2014/04/04 19:53:57]

Agreed, it reads a little badly. "didMoveFromOtherDocument" sgtm.

+    void didRemoveAllEventHandlers(EventTarget&);
+
+    // Registration of event handlers attached to non-DOM objects.
+    void didAddExternalEventHandler(const AtomicString& eventType);
+    void didRemoveExternalEventHandler(const AtomicString& eventType);
+private:
+    explicit EventHandlerRegistry(Document&);
+
+    enum ChangeOperation {
+        Add, // Add a new event handler.
+        Remove, // Remove an existing event handler.
+        RemoveAll // Remove any and all existing event handlers for a given target.
+    };
+
+    // Returns true if |eventType| belongs to a class this registry tracks.
+    static bool eventTypeToClass(const AtomicString& eventType, EventHandlerClass* result);
+
+    // Returns true if the operation actually added a new target or completely
+    // removed an existing one.
+    bool updateEventHandlerTargets(ChangeOperation, EventHandlerClass, EventTarget*);
+
+    // Called to notify clients whenever a single event handler target is
+    // registered or unregistered. If several handlers are registered for the
+    // same target, only the first registration will trigger this notification.
+    void notifyDidAddOrRemoveEventHandlerTarget(EventHandlerClass);
+
+    // Called on the EventHandlerRegistry of the root Document to notify
+    // clients when we have added the first handler or removed the last one for
+    // a given event class. |hasActiveHandlers| can be used to distinguish
+    // between the two cases.
+    void notifyHasHandlersChanged(EventHandlerClass, bool hasActiveHandlers);
+
+    // Record a change operation to a given event handler class and notify any
+    // parent registry and other clients accordingly.
+    void updateEventHandlerOfType(ChangeOperation, const AtomicString& eventType, EventTarget*);
+
+    void updateExternalHandlerCount(ChangeOperation, EventHandlerClass);
+    void updateEventHandlerInternal(ChangeOperation, EventHandlerClass, EventTarget*);
+
+    class DocumentObserver;
+    class WindowObserver;
+    friend class DocumentObserver;

[Rick Byers, 2014/04/03 15:33:19]

I thought nested class members were friends automatically?  It's not because
you're just forward declaring the type that you need to explicitly make it a
friend, is it?

[Sami, 2014/04/04 19:53:57]

I was thinking that was only the case for Java, but looks like it's true for C++
too since TR1. I'm not sure what language revision Chromium is targeting, but at
least GCC and Clang seem happy even without this line so I've removed it.

+
+    struct HandlerState {
+        HandlerState();
+        ~HandlerState();
+
+        OwnPtr<EventTargetSet> targets;
+        // External handlers are not included in |targets|.
+        unsigned externalHandlerCount;

[Rick Byers, 2014/04/03 15:33:19]

Now that you don't have handlerCount, it's tempting to consider simplifying this
further.  Eg. what if you just had a special singleton EventTarget instance that
you used to represent 'External'.  Then you could eliminate HandlerState
entirely and just have an EventTargetSet.  I think a bunch of your
implementation would collapse too.  It's up to you though - I don't know if
it'll ultimately be easier to understand that way or not. 

[Sami, 2014/04/04 19:53:57]

I see where you're going and it would simplify things somewhat. However, I'm
worried that clients of this class would find it odd to deal with an EventTarget
that's neither a Node nor a DOMWindow. That could lead to subtle bugs. Maybe the
right solution is to make WebViewImpl an EventTarget and get rid of external
handlers entirely.

[Rick Byers, 2014/04/04 20:12:27]

Yeah I wouldn't want to expose this to clients either - I was thinking of it
just as an implementation detail (and you could keep all the same public API
surface area and never let this special EventTarget leak out), but I guess that
would make the eventHandlerTargets implementation more complex.  So  maybe it's
not a net-win afterall.  I'm fine with it as is.

+    };
+
+    Document& m_document;
+
+    OwnPtr<DocumentObserver> m_documentObserver;

[Rick Byers, 2014/04/03 15:33:19]

Is there any value to the indirection via a pointer here (other than that it
makes this header file a little smaller by pushing the class declaration to the
cpp file)?  It would be a more direct transformation of what you were doing
before if you contained them by value.  No big deal though (eg. the perf
difference should be trivial) - your call...

[Sami, 2014/04/04 19:53:57]

I picked this way just to reduce the number of implementation details that leak
to the main header (and then forgot to remove the observer header includes from
there, oops). Since it turns out these observers aren't that complex in the end,
I'll just move them back to the header file.

[Rick Byers, 2014/04/04 20:12:27]

Ok.  Did you want to update m_windowObserver too?

+    OwnPtr<WindowObserver> m_windowObserver;
+
+    HandlerState m_eventHandlers[EventHandlerClassCount];
+};
+
+} // namespace WebCore
+
+#endif // EventHandlerRegistry_h